Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix crash when the expression exceed the depth #3606

Merged
merged 19 commits into from
Jan 6, 2022

Conversation

heroicNeZha
Copy link
Contributor

@heroicNeZha heroicNeZha commented Dec 30, 2021

What type of PR is this?

  • bug
  • feature
  • enhancement

What does this PR do?

closes #3393

Which issue(s)/PR(s) this PR relates to?

# when the expression exceed the depth
[ERROR (-1004)]: SyntaxError: The above expression is not a valid expression, because its depth exceeds the maximum depth! near `1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+'

calcate the depth in parser through BFS

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context/ Design document:

Checklist:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatibility (If it breaks the compatibility, please describe it and add the corresponding label.)
  • If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

                                                            `

@heroicNeZha heroicNeZha added ready for review ready-for-testing PR: ready for the CI test labels Dec 30, 2021
: expression_internal {
$$ = $1;
if(!graph::ExpressionUtils::checkExprDepth($$)){
throw nebula::GraphParser::syntax_error(@1, "The above expression's depth exceeds the maximum depth!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to delete this $$ before throw exception

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above expression's depth exceeds the maximum depth: 512!

ExpressionUtils::kMaxDepth

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

yixinglu
yixinglu previously approved these changes Dec 31, 2021
@Sophie-Xie Sophie-Xie added the cherry-pick-v3.0 PR: need cherry-pick to this version label Jan 4, 2022
jievince
jievince previously approved these changes Jan 4, 2022
Copy link
Contributor

@jievince jievince left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@heroicNeZha heroicNeZha dismissed stale reviews from jievince and yixinglu via 7aa7944 January 4, 2022 02:25
Copy link
Contributor Author

@heroicNeZha heroicNeZha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without depth check

============================================================================
/home/nebula/nebula/src/parser/test/ParserBenchmark.cpprelative  time/iter  iters/s
============================================================================
SimpleQuery(1_thread)                                       16.69us   59.93K
SimpleQuery(2_thread)                             99.27%    16.81us   59.49K
SimpleQuery(4_thread)                             99.38%    16.79us   59.56K
SimpleQuery(8_thread)                             99.17%    16.83us   59.43K
SimpleQuery(16_thread)                            99.00%    16.86us   59.33K
SimpleQuery(32_thread)                            95.46%    17.48us   57.21K
SimpleQuery(48_thread)                            86.43%    19.31us   51.80K
----------------------------------------------------------------------------
ComplexQuery(1_thread)                                      75.65us   13.22K
ComplexQuery(2_thread)                            99.68%    75.90us   13.18K
ComplexQuery(4_thread)                            99.51%    76.03us   13.15K
ComplexQuery(8_thread)                            99.51%    76.02us   13.15K
ComplexQuery(16_thread)                           98.88%    76.51us   13.07K
ComplexQuery(32_thread)                           95.30%    79.38us   12.60K
ComplexQuery(48_thread)                           85.81%    88.17us   11.34K
============================================================================

add depth check

============================================================================
/home/nebula/nebula/src/parser/test/ParserBenchmark.cpprelative  time/iter  iters/s
============================================================================
SimpleQuery(1_thread)                                       16.49us   60.65K
SimpleQuery(2_thread)                             99.38%    16.59us   60.27K
SimpleQuery(4_thread)                             99.22%    16.62us   60.17K
SimpleQuery(8_thread)                             99.20%    16.62us   60.16K
SimpleQuery(16_thread)                            98.25%    16.78us   59.58K
SimpleQuery(32_thread)                            95.33%    17.30us   57.81K
SimpleQuery(48_thread)                            85.46%    19.29us   51.83K
----------------------------------------------------------------------------
ComplexQuery(1_thread)                                      79.23us   12.62K
ComplexQuery(2_thread)                            99.81%    79.38us   12.60K
ComplexQuery(4_thread)                            99.88%    79.32us   12.61K
ComplexQuery(8_thread)                            99.85%    79.35us   12.60K
ComplexQuery(16_thread)                           99.28%    79.80us   12.53K
ComplexQuery(32_thread)                           96.18%    82.37us   12.14K
ComplexQuery(48_thread)                           84.38%    93.90us   10.65K
============================================================================

yixinglu
yixinglu previously approved these changes Jan 4, 2022
jievince
jievince previously approved these changes Jan 4, 2022
@yixinglu yixinglu merged commit a319936 into vesoft-inc:master Jan 6, 2022
critical27 added a commit that referenced this pull request Jan 10, 2022
* Fix typos (#3615)

Co-authored-by: kyle.cao <kyle.cao@vesoft.com>

* fix fetch edges tostring (#3613)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com>

* fix create space assign offline host (#3583)

* fix create space

* fix test case

Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com>

* Disable ARM version docker image since related third party not ready (#3618)

* Unify raft error code (#3620)

* Meta upgrader v3 (#3540)

* Replace group when create space

* Support white list

* fix test case

* support zone operations

* fix

* Support meta upgrade v3

* add more check about parse host result (#3628)

* Ut fix (#3611)

* Enable ut and fix chaindelete

* Add mock server default worker

* fix service crash (#3616)

* Cleanup branch param in package script (#3622)

* fix crash when the expression exceed the depth (#3606)

* Enhance login password check (#3629)

* fix_batch_insert_problem (#3627)

* filter data before batch insert

* add test cases

* add more testcase

* add notifyStop() for metaClient (#3621)

* add notifyStop() for metaClient

* do clean

* Fix removeSession() (#3651)

Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>

* Issue3373 storage exit crash (#3553)

* use rcu replace thread local

fix storage exit crash

format

address some comment

* fix bug

* fix bug

* Fix coalesce bug (#3653)

* fix coalesce

* fix test

* add test

* add tck

* fix

* fix

* fix

* delete double check agg in where clause (#3647)

Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
Co-authored-by: cpw <13495049+CPWstatic@users.noreply.github.com>

* fix meta crash after create space (#3660)

Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com>

Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com>
Co-authored-by: kyle.cao <kyle.cao@vesoft.com>
Co-authored-by: jimingquan <mingquan.ji@vesoft.com>
Co-authored-by: yaphet <4414314+darionyaphet@users.noreply.github.com>
Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com>
Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>
Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com>
Co-authored-by: endy.li <25311962+heroicNeZha@users.noreply.github.com>
Co-authored-by: lionel.liu@vesoft.com <52276794+liuyu85cn@users.noreply.github.com>
Co-authored-by: hs.zhang <22708345+cangfengzhs@users.noreply.github.com>
Co-authored-by: jakevin <30525741+jackwener@users.noreply.github.com>
Co-authored-by: cpw <13495049+CPWstatic@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-v3.0 PR: need cherry-pick to this version ready for review ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit the depth of the expression, report an error when it exceeds but not crash
6 participants